Skip to content

chg: 📌 use pip-tools and setup.py to pin all dependencies #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jan 13, 2022
Merged

chg: 📌 use pip-tools and setup.py to pin all dependencies #38

merged 3 commits into from
Jan 13, 2022

Conversation

@ianardee
Copy link
Contributor

@ianardee ianardee commented Jan 13, 2022 •
edited
Loading

Use pip-tools and setup.py to pin all dependencies

Description

Remove requirements.dev.txt and use setup.py
Use pip-tools to compile requirements.txt from setup.py, pinning dependencies

Related Issue

#23

Motivation and Context

Simplify installing dependencies and pin them to specific versions for security reasons.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@ianardee ianardee force-pushed the pin-dependencies branch 4 times, most recently from 18aeb30 to 5224907 Compare January 13, 2022 15:04
@fharper
Copy link
Contributor

fharper commented Jan 13, 2022

@fakela can you review the changes in the README only for this PR please?

fharper
fharper approved these changes Jan 13, 2022
View reviewed changes
Copy link
Contributor

@fharper fharper left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one small addition to the README, and letting time for @fakela to review the README modifications. But we can add those after if you need the dependencies PR to merge ASAP. Just let us know if you do @ianardee

@fharper
Copy link
Contributor

fharper commented Jan 13, 2022

@ianardee unless you are waiting for other reviewers, you can merge it. @fakela should review the whole README anyway, not just your changes.

@ianardee ianardee merged commit 95e815e into main Jan 13, 2022
@ianardee ianardee deleted the pin-dependencies branch January 13, 2022 20:23
@ianardee ianardee mentioned this pull request Jan 13, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ixunio ixunio Awaiting requested review from ixunio

@jonathanMindee jonathanMindee Awaiting requested review from jonathanMindee

1 more reviewer

@fharper fharper fharper approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ianardee @fharper